Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix rendering of enumerations inheriting from mixin and data types. #11596

Merged
merged 52 commits into from
Mar 21, 2024
Merged

Fix rendering of enumerations inheriting from mixin and data types. #11596

merged 52 commits into from
Mar 21, 2024

Conversation

picnixz
Copy link
Member

@picnixz picnixz commented Aug 15, 2023

Fix #11353.

I'm not sure that everything is covered so feel free to argue.

picnixz added 14 commits May 28, 2023 14:13
@picnixz
stash
4767ad3
@picnixz
Merge remote-tracking branch 'upstream/master' into fix/11353-autodoc…
b236d91 
…-multiple-inheritance-enum
@picnixz
update tests
b711e6b
@picnixz
add CHANGES
b09c0a3
@picnixz
update tests
c87bbac
@picnixz
Merge remote-tracking branch 'upstream/master' into fix/11353-autodoc…
54a6ea4 
…-multiple-inheritance-enum
@picnixz
Merge branch 'master' into fix/11353-autodoc-multiple-inheritance-enum
d2fa387
@picnixz
Merge branch 'master' into fix/11353-autodoc-multiple-inheritance-enum
14ecd55
@picnixz
Merge branch 'master' into fix/11353-autodoc-multiple-inheritance-enum
11b2964
@picnixz
Merge branch 'sphinx-doc:master' into fix/11353-autodoc-multiple-inhe…
d2d6301 
…ritance-enum
@picnixz
Merge branch 'sphinx-doc:master' into fix/11353-autodoc-multiple-inhe…
3f2c08c 
…ritance-enum
@picnixz
update CHANGES
3e6c602
@picnixz
Merge branch 'master' into fix/11353-autodoc-multiple-inheritance-enum
be9d9d2
@picnixz
Merge branch 'master' into fix/11353-autodoc-multiple-inheritance-enum
9662baa
@picnixz
Copy link
Member Author

picnixz commented Feb 25, 2024

@alxroyer Could you check if this patch would fix your issue?

@picnixz
Copy link
Member Author

picnixz commented Mar 19, 2024

Maybe convert to draft then 😄 ?

I fixed it actually, now I'm just cleaning it up.

@picnixz
Copy link
Member Author

picnixz commented Mar 19, 2024

So there is something with docutils but I don't know what.

@jayaddison can you investigate this one please?

@picnixz
Copy link
Member Author

picnixz commented Mar 19, 2024 •
edited
Loading

There's definitely something wrong with docutils HEAD. Maybe it's my fix that is causing the issue because it's really weird.

@chrisjsewell
Copy link
Member

There's definitely something wrong with docutils HEAD. Maybe it's my fix that is causing the issue because it's really weird.

Lokks like tests/test_builders/test_build_linkcheck.py::test_defaults is flaky, because I had the same failure on my PR (just re-run and it dissapeared)

@picnixz
Copy link
Member Author

picnixz commented Mar 19, 2024

Lokks like tests/test_builders/test_build_linkcheck.py::test_defaults is flaky, because I had the same failure on my PR (just re-run and it dissapeared)

It's funny because every time I rerun the test, it failed. So I am quite unlucky :(

@picnixz
Copy link
Member Author

picnixz commented Mar 19, 2024

I am really unlucky. I clearly don't know why this one became flaky now...

chrisjsewell
chrisjsewell requested changes Mar 19, 2024
View reviewed changes
Copy link
Member

@chrisjsewell chrisjsewell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the general logic looks reasonable, and the test cases capture a good range of possibilities.
Just some nitpicks.

tests/test_extensions/test_ext_autodoc.py Outdated Show resolved Hide resolved
sphinx/ext/autodoc/importer.py Show resolved Hide resolved
picnixz
picnixz commented Mar 20, 2024
View reviewed changes
Copy link
Member Author

@picnixz picnixz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm considering improving the logic of checking autodoc lines. For now I'll leave the current formatter as a base implementation but I'll review it in another PR so that we can easily match autodoc related content instead of having to write those lines with strings, indents and empty lines that are sometimes hard to remember.

sphinx/ext/autodoc/importer.py Outdated Show resolved Hide resolved
chrisjsewell
chrisjsewell approved these changes Mar 21, 2024
View reviewed changes
Copy link
Member

@chrisjsewell chrisjsewell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good thanks

@picnixz picnixz merged commit da0733b into sphinx-doc:master Mar 21, 2024
22 checks passed
@picnixz picnixz deleted the fix/11353-autodoc-multiple-inheritance-enum branch March 21, 2024 11:36
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2024
@AA-Turner AA-Turner added this to the 7.3.0 milestone Jul 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jayaddison jayaddison jayaddison approved these changes

@chrisjsewell chrisjsewell chrisjsewell approved these changes

@AA-Turner AA-Turner Awaiting requested review from AA-Turner

Assignees
No one assigned
Labels
extensions:autodoc
Projects
None yet
Milestone
7.3.0
Development

Successfully merging this pull request may close these issues.

Bad computation of enum superclass in sphinx.ext.autodoc.importer.get_class_members()
4 participants
@picnixz @chrisjsewell @AA-Turner @jayaddison